Conversation
| return ( | ||
| <div className="todo-card"> | ||
| <div className={`todo-card__check ${done ? "todo-card__check--done" : ""}`}> | ||
| <div |
There was a problem hiding this comment.
완료/미완료처럼 체크 상태를 바꾸는 기능이라면, 지금처럼 div에 클릭 이벤트를 다는 것보다는 input type=checkbox처럼 상태 의미가 드러나는 요소를 사용하는 편이 더 자연스럽습니다.
| onBlur, | ||
| }: TodoListProps) { | ||
| return ( | ||
| <div className="todo-list"> |
There was a problem hiding this comment.
이전에도 말씀드렸던 부분인데, 리스트를 보여주는 영역이라면 div 나열보다 ul과 li 구조로 맞춰두면 의미가 더 자연스럽습니다. 지금은 TodoCard가 반복 렌더링되고 있어서 리스트 마크업으로 바꾸기에도 잘 맞는 구조예요.
| white-space: nowrap; | ||
| color: #FFF; | ||
| text-align: center; | ||
| font-family: Pretendard; |
There was a problem hiding this comment.
CSS에서 Pretendard를 사용하고 있는데 현재 프로젝트에는 이 폰트를 실제로 불러오는 코드가 보이지 않아요. 의도한 폰트를 쓰고 싶다면 import까지 같이 넣어두면 더 좋겠습니다.
bk-git-hub
left a comment
bk-git-hub
left a comment
There was a problem hiding this comment.
수빈님 고생 많으셨습니다! 이번 PR에서는 useState로 입력값과 todo 배열을 연결하고, add/delete/toggle 흐름까지 자연스럽게 구현해주신 점이 좋았습니다. empty state까지 실제 동작 안에서 이어지도록 만든 점도 인상적이었어요.
다만 이전 리뷰에서 말씀드렸던 semantic list markup이나 폰트 import 같은 기본 구조 포인트는 아직 일부 남아 있습니다. 이번 리뷰에서는 체크 토글의 의미와 리스트 구조처럼 코드의 완성도를 조금 더 높여주는 부분 위주로 코멘트를 남겼습니다. 다음 PR에서는 이전 피드백 반영 여부도 같이 챙겨보시면 훨씬 더 탄탄해질 것 같습니다.
| <span className={`todo-card__text ${done ? "todo-card__text--done" : ""}`}> | ||
| {text} | ||
| </span> | ||
| <button className="delete-button" onClick={onDelete}> |
There was a problem hiding this comment.
이 삭제 버튼은 아이콘만 있어서 스크린리더 입장에서는 의미가 조금 모호할 수 있어요. aria-label 속성을 붙여두면 버튼의 의미가 더 분명해집니다.
2 participants
✅ 제출 정보
✅ 체크리스트 (필수)
main이 아니라 내 GitHub 핸들 브랜치(<handle>)다<handle>-week-xx형식이다🧩 구현 내용 요약
❓ 궁금한 점